fix: surface AMBIGUOUS_MATCH candidates and name find's supported actions - #1602
Conversation
…ions (#1597) AMBIGUOUS_MATCH errors now list the matching candidates (ref, role, label/identifier) rendered the same way as snapshot -i lines, capped at 5 with a "+N more" marker. buildAmbiguousMatchError (the single producer, src/daemon/handlers/find.ts) reuses formatSnapshotLine to build the list; formatAmbiguousMatchCandidateLines (src/utils/output.ts) renders it unconditionally on both text surfaces an agent actually reads (CLI printHumanError and MCP formatToolErrorText) — previously the candidates lived only in details, which neither surface printed. find's "Unsupported find action: X" (e.g. from `find <text> press`) now attaches a hint naming every action find actually supports and the two-step recovery shape: run find "<text>" to resolve the ref, then dispatch the gesture as its own command (press @enn). The hint is a single exported constant (UNSUPPORTED_FIND_ACTION_HINT) shared by both throw sites — packages/selectors' raw-token parser and the CLI's typed reader (src/commands/interaction/selectors.ts) — so they can't drift. Matching semantics are unchanged; ambiguous rejection stays by-design. The help-conformance corpus's AMBIGUOUS_MATCH quiz is updated: its premise ("candidate refs were not shown") no longer holds, but with 3 identically-labeled candidates the lesson (don't guess a specific ref) still holds.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks:
|
|
P2: The new candidate renderer is called unconditionally for every normalized error and stringifies any Exact-head Fallow is also owner-action red because |
…in shapes Review on #1602 (P2): formatAmbiguousMatchCandidateLines ran for every normalized error and stringified details.candidates unconditionally, but device-domain AMBIGUOUS_MATCH/APP_NOT_INSTALLED errors (findBootedAppleSimulatorWithApp, src/core/dispatch-resolve.ts) reuse that key for { id, name } device objects with no `matches` field — CLI and MCP would have printed "Candidates: [object Object]" for those. The renderer now requires numeric details.matches AND every candidate to be a string before rendering anything, restricting it to buildAmbiguousMatchError's element-match shape; unrecognized shapes render nothing, same as before this feature existed. Added regression tests against the exact device-error shape on both text surfaces. Also unexports AMBIGUOUS_MATCH_CANDIDATE_LIMIT (fallow flagged it as an unused production export) — it has no consumer outside find.ts.
|
Addressed both P2 findings in 724e603:
Re-ran 🤖 Addressed by Claude Code |
|
Re-review at |
thymikee
added
the
ready-for-human
|
1 participant
Summary
Fixes #1597 — two error-UX gaps where an agent got a bare error with nothing to act on.
1. AMBIGUOUS_MATCH now lists candidates.
buildAmbiguousMatchError(the single producer for element-match ambiguity —src/daemon/handlers/find.ts) now buildsdetails.candidatesfromformatSnapshotLine, the same compact renderersnapshot -iuses:@ref [role] "label/identifier". Capped at 5 (AMBIGUOUS_MATCH_CANDIDATE_LIMIT);details.matcheskeeps the true total.Critically, the candidates previously lived only in
details, which neither text surface an agent actually reads ever printed — the CLI'sprintHumanErrorand MCP'sformatToolErrorTextboth stopped atError: ...+Hint: .... A new shared renderer,formatAmbiguousMatchCandidateLines(src/utils/output.ts), prints the candidate lines unconditionally (not gated behind--debug) on both surfaces, with a+N moremarker when the cap trimmed the list:I audited every
AMBIGUOUS_MATCHemission site (grep -rn "AMBIGUOUS_MATCH") to confirmbuildAmbiguousMatchErrorreally is the one shared producer for element-match ambiguity — the other two throw sites (dispatch-resolve.tsdevice selection,session-doctor-app.tsapp selection) are a different domain (devices/apps, not elements) and already carry their own appropriate candidate shape. The direct-iOS-runner fast path forwait/get/iscan also raise a bare runner-nativeAMBIGUOUS_MATCH(ADR 0011, deliberately not delegated) — that one has no node list available at the point it's raised, so enriching it is out of scope here and its existing tests are untouched.2.
find <text> <action>with an unsupported action now hints at the fix.find <text> pressused to return a bareINVALID_ARGS: Unsupported find action: press. There were actually two throw sites for this (packages/selectors/src/internal/find.ts's raw-token parser, reachable from direct daemon/MCP dispatch, andsrc/commands/interaction/selectors.ts's typed CLI reader, reachable from the terminal) — both now attach the same hint via one shared exported constant,UNSUPPORTED_FIND_ACTION_HINT:Matching semantics are unchanged — ambiguous rejection stays by-design; this is purely an error-message/UX fix.
Red evidence
Before this change:
buildAmbiguousMatchErrorcapped candidates at 8 and formatted them as@e2(Follow)insidedetails.candidates— but neitherprintHumanErrornorformatToolErrorTextread that field at all, so the agent-visible text was justError (AMBIGUOUS_MATCH): find matched N elements... \n Hint: Multiple candidates matched...with no refs.Unsupported find action: presshad nodetails.hint, so it fell through to the genericINVALID_ARGSdefault hint ("Check command arguments and run --help for usage examples."), with no mention of what find does support or how to recover.New tests in
src/utils/__tests__/output.test.ts,src/mcp/__tests__/tool-error.test.ts,src/daemon/handlers/__tests__/find.test.ts,src/daemon/handlers/__tests__/find-args.test.ts, andsrc/commands/interaction/selectors.test.tsassert the new candidate lines / hint text directly and would fail against the old shapes.The help-conformance benchmark corpus's
AMBIGUOUS_MATCHquiz case previously taught models the opposite lesson on purpose ("candidate refs live in details, which the human rendering does not print"); it's updated to reflect that candidates now print, while keeping its core lesson intact (all 3 sample candidates share an identical label, so picking a specific@eNfrom this output alone is still a guess).Test plan
pnpm check:quick(lint + typecheck) — cleanpnpm check:layering— clean (addedUNSUPPORTED_FIND_ACTION_HINTto the pinned@agent-device/selectorsfaçade symbol list)pnpm format:check— cleanpnpm test:unit— 5297/5299 pass; the 2 failures (src/daemon/__tests__/runtime-hints.test.ts, unrelated Android/adb mocked tests) pass cleanly in isolation (15/15) — pre-existing timeout flakiness under parallel CPU contention, not touched by this change🤖 Generated with Claude Code